-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix templatizer under the template polyfill. forceDocumentUpgrade #2707
Conversation
8afc8f2
to
04550fe
Compare
We should be able to remove this code then: https://github.com/Polymer/polymer/blob/master/src/mini/template.html#L36-L42 |
LGTM with nit to remove the leftover template bootstrap code in |
was interacting problematically with un-prepped HTMLTemplateElements. By forcing a bootstrap before the document upgrade, we sidestep this issue.
04550fe
to
7c931ec
Compare
Notes for reasoning: The However, when using vulcanize with all the inlining flags, all elements will upgrade before the template polyfill runs, because of the call to This patch ensures that all templates in the document will be upgraded before all of the elements upgrade, just as in the standard use with HTMLImports. @sorvell @kevinpschaaf PTAL |
Reviewed along with @sorvell. Making the fix in I'm inclined to instead add specific code to bootstrap template extension elements in Polymer's startup flow, ala add this to if (this.localName == 'template') {
HTMLTemplateElement.decorate(this);
HTMLTemplateElement.bootstrap(this.content);
} This issue highlights a potentially serious performance issue with the way |
Per discussion, consider moving decorate & bootstrap code to CustomElements polyfill: https://github.com/webcomponents/webcomponentsjs/blob/master/src/CustomElements/upgrade.js#L60 |
I'm not sure I follow. Do you want the Template polyfill to actually use the CustomElement machinery? |
Just to be 100% clear, this PR should be closed and we will proceed with webcomponents/webcomponentsjs#439 right? |
Yes, that's the plan. |
was interacting problematically with un-prepped HTMLTemplateElements.
By forcing a bootstrap before the document upgrade, we sidestep this issue.